-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disable expandos for type-annotated function expressions in TS #61072
Disable expandos for type-annotated function expressions in TS #61072
Conversation
@typescript-bot test this |
Hey @sandersn, the results of running the DT tests are ready. There were interesting changes: Branch only errors:Package: codemirror
Package: styled-components
Package: hyper-function-component
|
@sandersn Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@typescript-bot user test this |
@jakebailey Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
@sandersn Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
@typescript-bot test this |
@sandersn Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
Hey @sandersn, the results of running the DT tests are ready. There were interesting changes: Branch only errors:Package: codemirror
Package: uikit
Package: styled-components
Package: react-currency-format
Package: hyper-function-component
|
@sandersn Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
@sandersn Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
@sandersn Here are some more interesting changes from running the top 400 repos suite Details
|
@sandersn Here are some more interesting changes from running the top 400 repos suite Details
|
Lots of usage of the un-annotated version. Plus in discussions after the design meeting, most people on the team think these semantics are reasonable for TS given that we've promised to support the same for function declarations. |
Thanks to Anders for noticing that this works in TS, but shouldn't. Unfortunately, he noticed in VSCode's repo, which makes me worried that it's widespread.
This only works in TS because assignment declarations from JS are incorrectly applied:
The correct way to write it is
We discussed whether the unannotated version makes sense in TS either, and I think we'd both prefer to remove it if we can.
Update: That's what the PR now implements; I'm running tests and will write analysis when they're done.
Results from removing Type-annotated only
There are 15 total failures, only 4 of which are React.FC usage. Most usages are local types, with only one or two from other projects. Overall I think that shows not very much usage of this feature. We should discuss removing it at another design meeting.
DT (3)
User tests (1)
top400 (12)
Results from removing all in TS
DT (5)
User tests (1)